fix(pool): close warm-pool shutdown-vs-replenish push race (orphaned-VM leak, MED) - #145
Merged
Merged
Conversation
…VM leak) From the concurrency audit. A freshly-booted warm VM could be pushed into the idle pool AFTER drain_idle already cleared it, leaking the VM (mount/IP/volume) since there is no Drop reaper. The replenish task checked `*shutdown_rx.borrow()` and only THEN acquired the idle lock to push — with an await in between. drain_all does signal_shutdown() then drain_idle() (which locks idle, drains, and runs exactly once). If the unlocked check observed shutdown=false and drain_idle completed before the task acquired the lock, the VM was pushed into the already-drained pool and never reclaimed. Fix: acquire the idle lock FIRST, then re-check shutdown UNDER it before pushing — making the check-and-push atomic against drain_idle (which holds the same lock while draining, always after signal_shutdown). On shutdown, destroy the VM instead of pushing. The same latent leak existed in `release()` (returns a VM to the pool with no shutdown check); guard it the same way. Validated: full a3s-box-runtime pool/lib suite (55) green, fmt + clippy clean. A minimal check-under-lock change; the interleaving itself needs a real-VM harness to exercise (like the kill-identity / durable-write fixes).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
From the concurrency audit. A freshly-booted warm VM could be pushed into the idle pool after
drain_idlealready cleared it, leaking the VM (overlay mount + IP + volumes) — there is noDropreaper.Race
The replenish task checked
*shutdown_rx.borrow()and only then acquired the idle lock to push — with anawaitin between:drain_alldoessignal_shutdown()thendrain_idle()(locks idle, drains, runs once). If the unlocked check sawfalseanddrain_idlecompleted before the task acquired the lock, the VM landed in the already-drained pool and was never reclaimed.Fix
Acquire the idle lock first, then re-check shutdown under it before pushing — making check-and-push atomic against
drain_idle(which holds the same lock while draining, always aftersignal_shutdown). On shutdown, destroy the VM instead of pushing. The same latent leak existed inrelease()(returns a VM to the pool with no shutdown check) — guarded the same way.Validation
Full
a3s-box-runtimepool/lib suite (55) green, fmt + clippy clean. A minimal check-under-lock change; the interleaving itself needs a real-VM harness to exercise (like the kill-identity / durable-write fixes).Third of the 4 concurrency findings (after #144's HIGH monitor-resurrect + MED kill-identity). Remaining: concurrent restart+monitor-boot → orphan VM (HIGH).